Cssfilter #477
Cssfilter #477
Conversation
|
Is there a change to |
| @@ -31,8 +31,8 @@ | |||
| from pylons.i18n import _ | |||
| from mako import filters | |||
|
|
|||
| import os | |||
| import tempfile | |||
| #import os | |||
There was a problem hiding this comment.
Good find of the unused imports, but could you just delete the lines please?
| } | ||
|
|
||
| cssutils.profile.addProfile("Reddit compat", custom_values, custom_macros) |
There was a problem hiding this comment.
Perhaps the profile name should be pulled out into a constant so it's not duplicated?
Nitpick: reddit should not be capitalized :)
There was a problem hiding this comment.
Can you explain what you mean by duplication?
There was a problem hiding this comment.
The string "Reddit compat" is written out twice in this code. If you wanted to change the name it'd require changing in two places.
|
I'm running this new code on every stylesheet in the database currently and so far I've found a few regressions:
I'll let you know if I come across more in my testing. That said, this is already proving its worth as it's found several places where the subreddit stylesheets were flat out wrong! (some |
|
Additionally, and I'm not sure if this is new behaviour or not, I don't appear to get line numbers with my validation errors. |
|
None of the three examples above are valid CSS. See the grammars at W3C [1] [2] [3]. The legacy code seems to be letting them through due to a custom rule. I can add that rule if you want to preserve the incorrect behavior. It looks like the new cssutils doesn't report line numbers for validation errors, only for parse errors. If it's important, I can look again more closely later this week. [1] http://www.w3.org/TR/css3-text/#text-shadow |
|
Both |
|
Ah, my mistake; I misinterpreted the spec. |
|
Alrighty, looks like that covers everything in the existing stylesheets that is actually valid. Gonna do a final code-review pass then we should be good to go! :) |
|
Unfortunately there is an infinite recursion bug in cssutils 0.9.10 holding this back. We'll follow up once that can be addressed. |
|
Thanks a lot, can't wait. (Hopefully not another 10 months xD). |
|
If it's still present, how can the infinite recursion bug be reproduced? |
|
Closing because this has been obviated by the replacement of cssutils with tinycss2. Thanks for the contribution and sorry for it not getting merged. |
Updated to the latest cssutils, remove duplicate code, and a few misc improvements.